-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Chore: Update 3rd Party libraries to the latest #4583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5fc91c8 to
dfda08e
Compare
| setContainer(instanceRef.current.contentDocument.body); | ||
| setWindow(() => instanceRef.current.contentWindow); | ||
| }, []); | ||
| }, [instanceRef]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will do anything because the value of instanceRef will never change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically the instanceRef.current can be changed during the mount/unmount... it is technically correct to have it there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But even when current changes, instanceRef will be the same object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True... And having it as a dependency doesn't hurt and is technically correct
| const handleRef = useCallback( | ||
| (ref: any) => { | ||
| instanceRef.current = { | ||
| contentDocument: ref ? ref.node.contentDocument : null, | ||
| contentWindow: ref ? ref.node.contentWindow : null, | ||
| }; | ||
| }, | ||
| [instanceRef], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it the instanceRef is now directly set and does the same thing
| const hasParentConst = isObject(parentConst) && (parentConst as JSONSchema7Object)[key] !== undefined; | ||
| const hasConst = | ||
| ((isObject(propertySchema) && CONST_KEY in propertySchema) || hasParentConst) && | ||
| ((typeof propertySchema === 'object' && CONST_KEY in propertySchema) || hasParentConst) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type checking. Our isObject() function did not work as a type guard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not update isObject to be a typeguard? Think it would be as simple as
export default function isObject(thing: any): thing is object {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, let me try that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works!
| /** Allows RJSF to override the default field implementation by specifying either the name of a field that is used | ||
| * to look up an implementation from the `fields` list or an actual one-off `Field` component implementation itself | ||
| */ | ||
| 'ui:field'?: Field<T, S, F> | string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, MakeUIType will handle this with field defined in UIOptionsBaseType
| }, | ||
| "devDependencies": { | ||
| "@ant-design/icons": "^5.6.1", | ||
| "@babel/cli": "^7.23.9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi, if we switch to pnpm then we may still need to explicitly list the dependencies we are using in each package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait until we do that then. I wanted to reduce the size of our package files right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had some questions, not blocking approval
Updated as many of the 3rd Party libraries as possible - Updated the root `package.json` to consolidate the dev dependencies from all of the sub-packages like: - Typescript, rollup, jest, react, types and babel - Also bumped all of the libraries that were out-of-date - Updated the individual `package.json` files to update libraries and to remove duplicated dev dependencies that are in the root `package.json` - Updated types in `@rjsf/utils` and `@rjsf/core` due to typescript and lodash type updates - Updated `DemoFrame` in `playground` to work with the latest version of `react-frame-component` - Updated all the snapshots for themes that needed them due to library updates
… problems with the latest version of Docusaurus
…ltFormState.ts` changes
136b121 to
1923b75
Compare
Reasons for making this change
Updated as many of the 3rd Party libraries as possible
package.jsonto consolidate the dev dependencies from all of the sub-packages like:package.jsonfiles to update libraries and to remove duplicated dev dependencies that are in the rootpackage.json@rjsf/utilsand@rjsf/coredue to typescript and lodash type updatesDemoFrameinplaygroundto work with the latest version ofreact-frame-componentdocslibraries, including fixing up .md files that caused problems with the latest version of Docusaurus@rjsf/utilsisObject()a type guardChecklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.